Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ElfSymbol::raw_symbol #562

Merged
merged 4 commits into from
Jul 24, 2023
Merged

Add ElfSymbol::raw_symbol #562

merged 4 commits into from
Jul 24, 2023

Conversation

koute
Copy link
Contributor

@koute koute commented Jul 24, 2023

I'm switching one of my projects from goblin to object, and getting access to raw ELF fields having an ElfSymbol32/ElfSymbol64 seems... frustratingly impossible? Is there any particular reason why the Sym trait isn't just simply implemented for ElfSymbol32 and ElfSymbol64, like I've done here in this PR?

@philipc
Copy link
Contributor

philipc commented Jul 24, 2023

getting access to raw ELF fields having an ElfSymbol32/ElfSymbol64 seems... frustratingly impossible

Yep, this is an area that needs improvement, and I'm still not sure which path to take. So far I've been adding things like this ad-hoc as people need them, but it desperately needs to be systematically solved for the entire crate.

Is there any particular reason why the Sym trait isn't just simply implemented for ElfSymbol32 and ElfSymbol64

I'm reluctant to do that because the Sym trait is at a different level of abstraction from ElfSymbol32 and ElfSymbol64, and because this isn't a solution for the entire crate. To do this for the entire crate, we would need to add another method for every field of every raw type.

An alternative to this is to add fn undecided_name(&self) -> &'data Sym32 for ElfSymbol32, or something like that, and that's closer to what we've been doing so far.

@koute
Copy link
Contributor Author

koute commented Jul 24, 2023

Okay, would something like this be better?

@koute koute changed the title Implement Sym for ElfSymbol32 and ElfSymbol64 Add ElfSymbol::as_sym Jul 24, 2023
philipc
philipc reviewed Jul 24, 2023
View reviewed changes
src/read/elf/symbol.rs Outdated Show resolved Hide resolved
@koute @philipc
Rename as_sym to raw_symbol
dd25716 
Co-authored-by: Philip Craig <[email protected]>
@philipc philipc merged commit 6da6c82 into gimli-rs:master Jul 24, 2023
@koute koute changed the title Add ElfSymbol::as_sym Add ElfSymbol::raw_symbol Jul 24, 2023
@koute
Copy link
Contributor Author

koute commented Jul 24, 2023

Thanks!

poliorcetics pushed a commit to JustRustThings/object that referenced this pull request Jul 24, 2023
@koute @poliorcetics
read/elf: add ElfSymbol::raw_symbol (gimli-rs#562)
3df103a
mcbegamerxx954 pushed a commit to mcbegamerxx954/object that referenced this pull request Jun 15, 2024
@koute
read/elf: add ElfSymbol::raw_symbol (gimli-rs#562)
55d1594
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipc philipc philipc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@koute @philipc